-
Notifications
You must be signed in to change notification settings - Fork 293
Add support for configuring runner through template #400
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
Signed-off-by: Leonidas Avdelas <[email protected]>
|
LGTM. I like it. It is not breaking anything but is just here as 3rd way to do it. I'd merge that as is. Even if you missed something in the template, it is not critical because things can be added if needed. Very good job! |
Signed-off-by: Leonidas Avdelas <[email protected]>
|
Sounds fair, I am removing it from draft state. |
|
I guess people will love this new feature. If you are keen in getting a maintainer of this repo, just say a word. |
|
Hey, thanks for the work on this, would it be possible to get a new release including this feature? |
|
@riemers cc |
|
I've tried to switch to the template using the master, thus only updating the dependency version, and setting
So maybe a new release should only be published once these points are clarified/fixed (I assume some/most points can be ignored, but some may need to be addressed, in case someone is having suggestion, I could look at making a PR). |
|
So I just realised that most of the parts I'm missing where set using the ansible-gitlab-runner/defaults/main.yml Lines 286 to 296 in 899d0cc
|
|
I've addressed the points I need via #401, seems to be OK: |
Yes, that was one of the issues I faced with this approach. We could consider parsing the original TOML to get this information specifically maybe, cause generally parsing the whole TOML feels quite complex. TBH I am not really sure what's the best approach on this.
Yes, indeed not every available option was included. Thank you for adding more of them.
I seem to have missed some of those or messed up with their naming, I will take a look and create a PR for them. |
This is a first attempt at creating the configuration from a template. I am not sure this would 100% work for all cases, so I would appreciate any feedback on how you would see this feature designed.
Closes #393